Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add chapter on relations from mastering plone (slighty updated) #1290

Merged
merged 10 commits into from
Sep 17, 2022
Merged

Conversation

pbauer
Copy link
Member

@pbauer pbauer commented Sep 4, 2022

No description provided.

@netlify
Copy link

netlify bot commented Sep 4, 2022

Deploy Preview for 6-dev-docs-plone-org ready!

Name Link
🔨 Latest commit 63b521e
🔍 Latest deploy log https://app.netlify.com/sites/6-dev-docs-plone-org/deploys/63231f189a3b080007103712
😎 Deploy Preview https://deploy-preview-1290--6-dev-docs-plone-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@ksuess ksuess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From time to time I think it would be good to stop defining target sets of a relation field in its widget. Cause this is a constraint of its data structure (Plone: schema) and not a matter of the display (Plone: widget).

docs/backend/relations.md Outdated Show resolved Hide resolved
@pbauer pbauer requested a review from stevepiercy September 5, 2022 07:38
@stevepiercy
Copy link
Contributor

I will review this in the next day or two. I'm working on other docs at the moment. Please do not merge yet. Thank you!

@stevepiercy
Copy link
Contributor

General comment, with details coming in a full review later.

Documentation should be the authoritative source, not Training. We should pull content from the Training, and insert and adapt it in Documentation.

If that is too much work for now, then I would be OK with leaving the seealso directives on the previously empty pages (for example, docs/backend/behaviors.md) because it is an improvement, giving the reader something to use as a reference. We should also add a todo directive for each of these pages, and create a new issue in the Documentation issue tracker for each one.

I also think that this would give some newbies an opportunity to contribute by repurposing Training material into Documentation. Or so I hope.

@pbauer what do you think?

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my first round of review.

I did not start on the mammoth backend/relations.md, but I will start nibbling on that pachyderm one bite at a time over the next day or so.

docs/backend/control-panels.md Outdated Show resolved Hide resolved
docs/backend/vocabularies.md Outdated Show resolved Hide resolved
docs/backend/widgets.md Show resolved Hide resolved
@pbauer
Copy link
Member Author

pbauer commented Sep 7, 2022

Documentation should be the authoritative source, not Training. We should pull content from the Training, and insert and adapt it in Documentation.

Yes. The chapters on relations, and the dexterity-reference can be moved to the docs without mayor changes. I'll remove them from the training and link to the docs once these are merged.

If that is too much work for now, then I would be OK with leaving the seealso directives on the previously empty pages (for example, docs/backend/behaviors.md) because it is an improvement, giving the reader something to use as a reference. We should also add a todo directive for each of these pages, and create a new issue in the Documentation issue tracker for each one.

Yes

I also think that this would give some newbies an opportunity to contribute by repurposing Training material into Documentation. Or so I hope.

You can always hope but I doubt it.

@stevepiercy
Copy link
Contributor

Added meta data and TODOs in 443a010.

@stevepiercy
Copy link
Contributor

I'm going to continue editing and pushing commits to this tomorrow. The commits I pushed so far are just meta and technical stuff that we should include in docs.

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is excellent work. To save review cycles, I made lots of nit-picky commits to clean up English and MyST syntax and grammar.

I have a few additional questions. Please see the comments and suggestions.

Thank you!

docs/backend/relations.md Outdated Show resolved Hide resolved
docs/backend/relations.md Outdated Show resolved Hide resolved
docs/backend/relations.md Outdated Show resolved Hide resolved
docs/backend/relations.md Show resolved Hide resolved
docs/backend/relations.md Outdated Show resolved Hide resolved
docs/backend/relations.md Outdated Show resolved Hide resolved
docs/backend/relations.md Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants